Enforce max-ref-age-ms when expiring snapshots by 1fanwang · Pull Request #3760 · apache/iceberg-python

1fanwang · 2026-08-07T09:10:52Z

Rationale for this change

create_branch() and create_tag() accept max_ref_age_ms and write it to table metadata, but nothing in pyiceberg acts on it.

That leaks storage. A live ref protects its snapshot from expiry, so a stale ref pins that snapshot and its ancestors indefinitely.

The spec makes ref removal step 2 of the snapshot retention policy:

Remove any refs (other than main) where the referenced snapshot is older than max-ref-age-ms

Java implements this in RemoveSnapshots.computeRetainedRefs().

This adds ExpireSnapshots.remove_expired_refs() with the same semantics: a ref's age comes from the timestamp of the snapshot it points at, compared against its own max-ref-age-ms or the new history.expire.max-ref-age-ms table property. main never expires, and a ref whose snapshot is gone is removed. It is opt-in, matching the existing builder idiom, so current behavior is unchanged.

older_than() now resolves its snapshot set at commit time. Otherwise older_than(dt).remove_expired_refs() would drop the ref but keep the snapshot it had pinned, since that ref was still protected when older_than() ran. The two calls are now order-independent.

Out of scope: spec steps 4 and 5, max-snapshot-age-ms, and min-snapshots-to-keep during ancestor traversal.

Prior art

#3246 proposed this in April and was closed by the stale bot without review. This PR uses the same design. It differs by reading the table property the spec names as the default instead of taking a required argument, and by using a UTC clock.

Are these changes tested?

Integration tests in tests/integration/test_snapshot_operations.py run against the REST catalog and Hive metastore from dev/docker-compose-integration.yml. They cover an expired branch being removed and its snapshot reclaimed, plus a branch inside its retention window surviving and continuing to protect its snapshot.

test_remove_expired_refs[session_catalog_hive] PASSED
test_remove_expired_refs[session_catalog] PASSED
test_remove_expired_refs_keeps_unexpired_branch[session_catalog_hive] PASSED
test_remove_expired_refs_keeps_unexpired_branch[session_catalog] PASSED

Against unpatched pyiceberg/ these four fail with AttributeError: 'ExpireSnapshots' object has no attribute 'remove_expired_refs'.

Unit coverage in tests/table/test_expire_snapshots.py covers the memory, sql, and sql_without_rowcount catalogs: expired branch removed, unexpired branch kept, table-property fallback, main exempt, order independence, and behavior unchanged without the opt-in.

prek run -a is clean.

Are there any user-facing changes?

Additive:

ExpireSnapshots.remove_expired_refs().
TableProperties.MAX_REF_AGE_MS (history.expire.max-ref-age-ms), defaulting to no expiry, matching Java's Long.MAX_VALUE.
An "Expiring Branches and Tags" section in mkdocs/docs/api.md.

Refs are removed only when remove_expired_refs() is called explicitly.

Copilot

Pull request overview

This PR adds support for expiring stale snapshot refs (branches/tags) based on max-ref-age-ms, bringing PyIceberg’s snapshot expiration behavior closer to Iceberg’s retention policy semantics and preventing refs from pinning snapshots indefinitely.

Changes:

Add ExpireSnapshots.remove_expired_refs() to drop stale refs based on per-ref max-ref-age-ms with fallback to history.expire.max-ref-age-ms.
Make older_than() resolve at commit time so chaining order with remove_expired_refs() does not affect which snapshots become eligible for expiration.
Add unit tests and API documentation for expiring branches/tags and the new table property.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
tests/table/test_expire_snapshots.py	Adds coverage for ref expiry semantics (expired/unexpired refs, opt-in behavior, order independence, table-property fallback, main exemption).
pyiceberg/table/update/snapshot.py	Implements ref expiry staging and defers `older_than()` evaluation to commit to make builder chaining order-independent.
pyiceberg/table/init.py	Introduces `TableProperties.MAX_REF_AGE_MS` and its default.
mkdocs/docs/api.md	Documents how to expire branches/tags and reclaim pinned snapshots.

Suppressed comments (2)

tests/table/test_expire_snapshots.py:409

This test creates a fixed namespace name in a session-scoped warehouse; if another test already created it, create_namespace will raise NamespaceAlreadyExistsError. Use create_namespace_if_not_exists (or randomize the namespace) to keep the suite order-independent.

    catalog_with_warehouse.create_namespace("expire_refs")

tests/table/test_expire_snapshots.py:433

This test creates a fixed namespace name in a session-scoped warehouse; if another test already created it, create_namespace will raise NamespaceAlreadyExistsError. Use create_namespace_if_not_exists (or randomize the namespace) to keep the suite order-independent.

    catalog_with_warehouse.create_namespace("expire_refs")

1fanwang · 2026-08-08T04:11:14Z

+    MAX_REF_AGE_MS = "history.expire.max-ref-age-ms"
+    MAX_REF_AGE_MS_DEFAULT = sys.maxsize


Fixed in ca43097. sys.maxsize is 2**31-1 on a 32-bit build, so the default would have been ~25 days rather than unbounded — pinned to 2**63-1 to match Java's Long.MAX_VALUE.

1fanwang · 2026-08-08T04:11:22Z

+
+    Returns the reloaded table and the snapshot id the branch pins.
+    """
+    catalog_with_warehouse.create_namespace("expire_refs")


Fixed in ca43097 — each test now uses its own namespace. The collision does not actually occur today, since catalog_with_warehouse calls destroy_tables() on teardown, but the tests should not depend on that for isolation.

create_branch() and create_tag() accept max_ref_age_ms and write it to table metadata, but nothing in pyiceberg acts on it. Since a live ref protects its snapshot from expiry, a stale ref pins that snapshot and its ancestors indefinitely. Add ExpireSnapshots.remove_expired_refs(), implementing step 2 of the spec's snapshot retention policy: drop refs other than main whose referenced snapshot is older than max-ref-age-ms, falling back to the new history.expire.max-ref-age-ms table property. Refs pointing at a snapshot that no longer exists are removed too. older_than() resolves its snapshot set at commit time so that snapshots released by remove_expired_refs() are reclaimed in the same commit regardless of the order the two are chained in. Signed-off-by: 1fanwang <1fannnw@gmail.com>

Copilot AI lite review requested due to automatic review settings August 7, 2026 09:10

Copilot started reviewing on behalf of 1fanwang August 7, 2026 09:12 View session

Copilot AI reviewed Aug 7, 2026

View reviewed changes

1fanwang force-pushed the fix-expire-refs-max-ref-age branch from 7a86150 to ca43097 Compare August 7, 2026 17:45

1fanwang mentioned this pull request Aug 8, 2026

Add ManageSnapshots.cherry_pick_snapshot #3769

Open

Navigation Menu

Uh oh!

Enforce max-ref-age-ms when expiring snapshots - #3760

Enforce max-ref-age-ms when expiring snapshots#3760
1fanwang wants to merge 1 commit into
apache:mainfrom
1fanwang:fix-expire-refs-max-ref-age

1fanwang commented Aug 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

1fanwang Aug 8, 2026

Uh oh!

1fanwang Aug 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		MAX_REF_AGE_MS = "history.expire.max-ref-age-ms"
		MAX_REF_AGE_MS_DEFAULT = sys.maxsize

Uh oh!

Conversation

1fanwang commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

Prior art

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

1fanwang Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

1fanwang Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

1fanwang commented Aug 7, 2026 •

edited

Loading